refactor: webhook への送信と再送を Webhook::Client にまとめる - #124
Merged
Conversation
Slack と Discord のアダプタは投稿の組み立て方こそ違うが、「webhook URL に JSON を POST し、一時的な失敗は再送する」点は同じで、再送の判断と待機が二重に書かれていた (issue #123)。片方だけ直すと挙動がずれるため、送信そのものを切り出す。 まず受け皿となる Webhook::Client を追加する。アダプタの移行は次のコミットで行う。 送信の実体を post_json に分けてあるので、HTTP を張らずに再送の判断を試せる。 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SmoDC7XRgDx1AcWumgv3Uq
両アダプタから再送ループ、Retry-After の解釈、リトライ回数と待機上限の定数を 取り除き、Webhook::Client に委ねる(issue #123)。アダプタに残るのは投稿の 組み立てと、Discord のペイロード記録(issue #95)だけになる。 エラーメッセージの送信先名はクライアントに渡す service で保つため、 `slack webhook returned ...` / `discord webhook returned ...` は変わらない。 Slack はこれまで Content-Type を付けずに送っていたが、共通化にあわせて Discord と 同じく application/json を付ける。Slack の Incoming Webhook が案内している送り方に 揃える形になる。 spec ではクライアントを差し替えられるよう、Webhook::Client を受け取る初期化を 足した。アダプタ側の spec は投稿の分かれ方と yield の有無だけを見る。 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SmoDC7XRgDx1AcWumgv3Uq
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
issue #123 の対応。
やったこと
Slack と Discord のアダプタは投稿の組み立て方こそ違うが、「webhook URL に JSON を POST し、一時的な失敗は再送する」点は同じで、再送の判断と待機が二重に書かれていた。
片方だけ直すと挙動がずれるため、送信そのものを
Webhook::Clientに切り出した。重複していたのは次の 3 つで、すべてクライアント側に移した。
MAX_SEND_ATTEMPTS/MAX_RETRY_WAITの定数Retry-Afterに従って再送し、それ以外は例外にする送信ループRetry-Afterを読んで待機上限で丸めるretry_afterアダプタに残るのは投稿の組み立てと、Discord のペイロード記録(#95)だけになった。
変更点
Webhook::Clientを追加した(1 コミット目)src/webhook/client.crに置いた。post(body)は送信できたら戻り、恒久的に失敗したら例外にする。送信の実体は
post_jsonに分けてある。ここだけを差し替えれば、HTTP を張らずに再送の判断を試せる。
Slack と Discord をクライアントに寄せた(2 コミット目)
両アダプタから再送ループと定数を取り除いた。
エラーメッセージの送信先名はクライアントに渡す
serviceで保つため、slack webhook returned .../discord webhook returned ...は変わらない。spec からクライアントを差し替えられるよう、
Webhook::Clientを受け取る初期化を足した。main.crが使う URL 版の初期化はそのまま残してある。挙動が変わる点
Slack はこれまで
Content-Typeを付けずに送っていたが、共通化にあわせて Discord と同じくapplication/jsonを付ける。Slack の Incoming Webhook が案内している送り方に揃える形になる。
それ以外は変えていない。
再送の条件(429 と 5xx)、試行回数(3 回)、待機上限(5 秒)、
Retry-Afterが無いときの既定(1 秒)はどちらも従来どおり。テスト
crystal specは 124 examples, 0 failures。再送の判断は
spec/webhook/client_spec.crに移した。Retry-Afterが無い応答でも待機上限を超えないことアダプタ側の spec は投稿の分かれ方と yield の有無だけを見る。
spec/slack/repository_spec.cr: 全メッセージを 1 投稿で送り累計を yield すること、失敗時は yield しないことspec/discord/repository_spec.cr: 12 件なら 2 投稿に分かれ投稿ごとに累計を yield すること、失敗した投稿では yield しないこと(新規。リファクタで壊れていないことを見るために足した)検証環境について
前回(#122)と同じく、ネットワーク制限により Crystal 1.21.0 を用意できず Ubuntu の 1.11.2 で検証している。
crystal tool formatは 1.11 と 1.21 で複数行引数の末尾カンマの扱いが違いリポジトリ全体に差分を出すため実行せず、追加・変更した 6 ファイルだけを個別に確認した。ameba は 1.11 でビルドできないためローカル実行できていない。
Closes #123
Generated by Claude Code